refactor: extract album buffer from telegram bot - #162
Merged
Conversation
- Extract _download_and_validate_photo, replacing the same get_file/download/validate block in photo and album handling - Extract _run_sequenced_turn, replacing an identical cancel/wait/track block in _safe_handle_update and _process_flushed_album - Extract _run_turn_and_send_response, replacing the same load-profile/run-turn/send-response sequence duplicated across 5 call sites - Extract _chunk_buttons, replacing identical keyboard-row chunking in the model and thinking settings menus - No behavior change; full test suite (1558 tests) passes unchanged
- Move the /model and /thinking inline-keyboard settings UI (menu building, callback parsing, capability resolution, reasoning options) out of TelegramBot into a standalone SettingsMenu class in telegram/settings_menu.py - TelegramBot now owns a SettingsMenu instance and delegates command dispatch and non-health callback routing to it; _load_chat_profile stays on TelegramBot since turn handling needs it too - bot.py drops from 1891 to 1500 lines; the extracted ~450 lines gain their own module instead of sharing an unrelated God class - Rewrite the coupled test suite: settings-menu behavior now tests SettingsMenu directly in tests/test_telegram_settings_menu.py; the old test_telegram_bot_model_override.py is trimmed to the thin TelegramBot-level delegation tests - No behavior change; full test suite (1560 tests) passes
- Move photo-album buffering (debounce/max-wait timers, the buffer dict, _BufferedAlbum) out of TelegramBot into a standalone AlbumBuffer class in telegram/album_buffer.py - TelegramBot owns an AlbumBuffer instance, threads its shared update-sequence counter into add_message(), and registers a callback to schedule turn processing on flush; the actual turn logic (_process_flushed_album, _handle_album_turn) stays on TelegramBot since it needs session/turn machinery - bot.py drops from 1500 to 1382 lines - Split the coupled test suite: pure buffering-mechanics tests now target AlbumBuffer directly in tests/test_telegram_album_buffer.py; the remaining end-to-end album tests in test_telegram_bot.py are updated to the new attribute paths (bot._album_buffer._buffers, etc.) - No behavior change; full test suite (1560 tests) passes
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
QueryPlanner
force-pushed
the
refactor/extract-settings-menu
branch
from
August 19, 2026 05:32
02ebfdb to
18dcb67
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Extracts the photo-album (media-group) buffering mechanics out of
TelegramBotinto a standaloneAlbumBufferclass in a newtelegram/album_buffer.py. Third and final PR in the cleanup series fortelegram/bot.py(stacked on #161, which is stacked on #160 — please review/merge in order).Why
Telegram delivers a multi-photo album as several separate messages sharing a
media_group_idrather than one message, so the bot has to buffer them, wait for a short debounce window (or a max-wait cap) for the rest to arrive, then process them as one turn. That buffering logic (~150 lines: a buffer dict, debounce/max-wait timers, the_BufferedAlbumdataclass) was one of five distinct concerns bolted onto the sameTelegramBotclass identified in the original audit.As with #161, the existing test coverage for this subsystem pokes
TelegramBot's private attributes directly (bot._album_buffers,bot._flush_album(...), etc.) across a ~1300-line test class intest_telegram_bot.py, so the clean extraction required updating that test coverage too, per the earlier "full extraction + test rewrite" agreement. Turned out simpler than the settings-menu case: most of these tests are genuine end-to-end integration tests throughTelegramBot._safe_handle_update(message arrives → buffered → flushed → turn processed → response sent) and only needed mechanical attribute-path updates, not a conceptual rewrite. Only 4 of ~30 tests were pure buffering-mechanics tests that made sense to move to their own file targetingAlbumBufferdirectly.Like #161, this is a relocation for separation of concerns, not a line-count reduction (that was #160's job) —
bot.pyshrinks by 118 lines, extracted into its own module plus a proportionally small new test file.How
telegram/album_buffer.py:_BufferedAlbumdataclass andAlbumBufferclass owning the buffer dict, debounce/max-wait constants, and all buffering-mechanics methods, moved verbatim fromTelegramBot.AlbumBuffertakes anon_flushcallback invoked synchronously with the completed album — it owns no turn-processing or session logic itself.bot.py:TelegramBot.__init__constructsself._album_buffer = AlbumBuffer(on_flush=self._on_album_flushed); the shared_update_counter(used to interleave cancellation ordering between album and non-album turns for the same conversation) stays onTelegramBotand is threaded intoadd_message(message, seq)explicitly, since it can't be split across two counters without breaking that ordering.stop()now callsawait self._album_buffer.shutdown()._process_flushed_albumand_handle_album_turn(the actual turn-processing business logic — session identity, downloading photos, running the ADK turn) stay onTelegramBotsince they need its session/turn machinery; only the buffering mechanics moved.bot.py(1500 → 1382 lines); no behavior change, verified by porting every existing test case.tests/test_telegram_album_buffer.py(new, 144 lines) coversAlbumBuffer's buffering state machine in isolation (cleanup branches, debounce/max-wait triggering flush, already-processed guard).tests/test_telegram_bot.pykeeps its ~26 end-to-end album integration tests, updated to the new attribute paths (bot._album_buffer._buffers,bot._album_buffer._flush(...),bot._album_buffer.cleanup(...),bot._album_buffer._max_wait(...), and the_BufferedAlbumimport moved toblacki.telegram.album_buffer).Tests
pytest tests/test_telegram_album_buffer.py tests/test_telegram_bot.py -q— 242 passedpytest -q(full suite) — 1560 passed, 1 skippedruff check/ruff format --checkon all changed/new files — cleanmypyonsrc/blacki/telegram/— clean